TDL-14896: Make batch_size apply to the first page and TDL-14895: Use the pagination__list_subscriber_interval_quantity config value correctly #64

hpatel41 · 2021-08-23T12:00:12Z

Description of change

TDL-14896: Make batch_size apply to the first page

Added code change to respect batch_size for 1sy page sync.

TDL-14895: Use the pagination__list_subscriber_interval_quantity config value correctly

Corrected the spelling of 'pagination__list_subscriber_interval_quantity' to use for syncing.

Manual QA steps

Risks

Rollback steps

revert this branch

dbshah1212 · 2021-08-26T10:02:43Z

tap_exacttarget/endpoints/campaigns.py

@@ -38,10 +38,15 @@ class CampaignDataAccessObject(DataAccessObject):
    KEY_PROPERTIES = ['id']

    def sync_data(self):
+        batch_size = int(self.config.get('batch_size', 2500))


As we are using this variable in multiple places, can we create a one base file?

dbshah1212 · 2021-08-30T12:08:05Z

tap_exacttarget/dao.py

@@ -19,6 +19,7 @@ def __init__(self, config, state, auth_stub, catalog):
        self.state = state.copy()
        self.catalog = catalog
        self.auth_stub = auth_stub
+        self.batch_size = int(self.config.get('batch_size', 2500))


Can we use self.batch_size here?

No, here the batch size is initialized.

dbshah1212 · 2021-08-30T12:12:26Z

tests/test_exacttarget_pagination.py

+
+                # collect information for assertions from sync based on expected values
+                record_count_sync = sync_record_count.get(stream, 0)
+                primary_keys_list = [(message.get('data').get(expected_pk) for expected_pk in expected_primary_keys)


Instead of this use: tuple([message['data']['expected_pk'] for expected_pk in expected_primary_keys]) as first comprehension here that iterates over the expected_primary_keys, is just making generators.

dbshah1212 · 2021-08-30T12:14:13Z

tests/test_exacttarget_pagination.py

+        return props
+
+    def test_run(self):
+        page_size = 1


If we have records then use at least 2 instead of one to test pagination properly.

Email has 2 records only. So, we cannot use page size 2.

…cross pages

KrisPersonal · 2021-10-12T18:26:47Z

tap_exacttarget/client.py

@@ -7,13 +7,13 @@
 LOGGER = singer.get_logger()


-def _get_response_items(response):
+def _get_response_items(response, name):


Please add comments to the changes

Added comments

KrisPersonal · 2021-10-12T18:27:18Z

tap_exacttarget/client.py

@@ -107,6 +107,8 @@ def request(name, selector, auth_stub, search_filter=None, props=None, batch_siz
    """
    cursor = selector()
    cursor.auth_stub = auth_stub
+    # set batch size
+    cursor.options = {"BatchSize": batch_size}


Please add comments regarding the changes

Added comment

KrisPersonal · 2021-10-12T18:27:47Z

tap_exacttarget/client.py

+        if isinstance(cursor, FuelSDK.ET_Campaign):
+            # use 'getMoreResults' for campaigns as it does not use
+            # batch_size, rather it uses $page and $pageSize and REST Call
+            response = cursor.getMoreResults()
+        else:
+            # Override call to getMoreResults to add a batch_size parameter
+            # response = cursor.getMoreResults()
+            response = tap_exacttarget__getMoreResults(cursor, batch_size=batch_size)


What is the purpose of the above changes?

The purpose of the above change is to support pagination in the tap to run the pagination test.

…nResetErrors (#71) * added backoff for certain errors * resolve pylint * updated decorator location * added unittests * added comment * added comments

* updated error message when generating auth_stub * made changes according to the comments * updated the code acording to the comments * updated the tap tester image * updated pylint and astroid to latest version * updated the code as, on updating tap tester image it was throwing cascading errors * updated config.yml file * updated the start date for integration tests as per the params * removed scenario as new tap-tester version does not support it * updated start date in the base file test

* added best practices * resolve pylint * resolve test failure * test: updated the test cases * test: updated some test cases * updated the code as per comments * resolve test case failure * updated the code as per comments * resolve integration test failure * resolve tes case failure * resolve test case failure * add data extension stream in test cases * added data extension incremental stream * test: run bookmark test * test: run bookmark test * test: run bookmark test, debug failing test * test: pylint resolve * test: run all data extenstion stream test in bookmark test * test: updated data extension code * updated the test to run data extension stream * run all tests * added sys.exit and updated pylint and astroid to latest versions * resolve pylint * updated the files as per comments * updated the code

…stream does not bookmark correctly (#75) * make keys automatic * pylint resolve * add full replication test case * added code change for data extension stream * pylint resolve * added comment * added comments * added comment in base file * updated discovery test and removed full replication test * updated the code * added a comment explaining subscriber and list subscriber syncing * added comments

* make keys automatic * pylint resolve * add full replication test case * use transformation * resolve pylint * added code change for data extension stream * pylint resolve * updated test case for data extension * added comment * added comments * added comments and optimized that condition * added code change for tranformation function in base file * pylint resolve * disabled pylint error * test: removed disable pylint code * added comment in base file * updated comment * updated the comment for skipping streams * updated discovery test and removed full replication test * added verification of unique records * updated start date * updated the code * updated the code * added a comment explaining subscriber and list subscriber syncing * added comments * updated comment * made separate files for schemas * resolve pylint * resolve integration test * corrected typo * updated the code * resolved unittest case error

* added code change to use start date when state is not passed * updated code based on comments * resolve test case failure * added data extension stream in tests * add data extension incremental stream * added future date test * updated start date test case * updated future start date test code * updated the code * added comment * added more comments in the code * resolve unittests failure * resolve integration test failure

kspeer825

Can this PR be closed? It looks like the changes may have been covered elsewhere.

added code change to respect batch size for 1st page sync

6c25a28

dbshah1212 reviewed Aug 26, 2021

View reviewed changes

harshpatel4_crest added 2 commits August 27, 2021 14:52

updated code according to comment

623d248

resolve pylint error

50689ba

hpatel41 requested a review from dbshah1212 August 27, 2021 09:34

hpatel41 marked this pull request as ready for review August 27, 2021 09:34

hpatel41 requested review from karanpanchal-crest and savan-chovatiya August 27, 2021 09:34

dbshah1212 reviewed Aug 30, 2021

View reviewed changes

harshpatel4_crest added 8 commits September 1, 2021 16:05

updated code based on comments

a71197c

resolve test cases failure

3e4c22c

added data extension stream in the test

f069f96

added batch size for data extension stream

1d23a24

pylint resolve

e1acd6a

updated logger messages

dbeeb6b

pylint resolve

054acac

added data extension incremental stream

73aeb95

karanpanchal-crest approved these changes Sep 8, 2021

View reviewed changes

hpatel41 requested review from cosimon, KAllan357 and kspeer825 September 13, 2021 13:03

harshpatel4_crest added 4 commits September 16, 2021 12:50

added assertion for verifying that we did not duplicate any records a…

e353b85

…cross pages

updated the code

1f56470

updated the code

520b087

resolve unittest failure

58464cc

hpatel41 requested review from luandy64 and zachharris1 September 23, 2021 11:10

updated comments

e38f478

KrisPersonal requested changes Oct 12, 2021

View reviewed changes

added comments

7a9f289

hpatel41 requested a review from KrisPersonal October 13, 2021 06:03

hpatel41 added 5 commits October 13, 2021 11:44

TDL-14621: Add retry logic to requests and TDL-14622: Retry Connectio…

0418198

…nResetErrors (#71) * added backoff for certain errors * resolve pylint * updated decorator location * added unittests * added comment * added comments

KrisPersonal approved these changes Oct 13, 2021

View reviewed changes

hpatel41 and others added 3 commits October 13, 2021 23:37

resolve merge conflicts: crest-master branch

e278f8c

resolve unittests failure

4cc3fc1

hpatel41 mentioned this pull request Oct 13, 2021

Crest Work to Merge #72

Merged

KrisPersonal mentioned this pull request Oct 14, 2021

Bump to version 1.7.0 #80

Merged

kspeer825 reviewed Nov 15, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TDL-14896: Make batch_size apply to the first page and TDL-14895: Use the pagination__list_subscriber_interval_quantity config value correctly #64

TDL-14896: Make batch_size apply to the first page and TDL-14895: Use the pagination__list_subscriber_interval_quantity config value correctly #64

hpatel41 commented Aug 23, 2021

dbshah1212 Aug 26, 2021

hpatel41 Aug 27, 2021

dbshah1212 Aug 30, 2021 •

edited

Loading

hpatel41 Sep 24, 2021

dbshah1212 Aug 30, 2021

hpatel41 Sep 24, 2021

dbshah1212 Aug 30, 2021

hpatel41 Sep 24, 2021

KrisPersonal Oct 12, 2021

hpatel41 Oct 13, 2021

KrisPersonal Oct 12, 2021

hpatel41 Oct 13, 2021

KrisPersonal Oct 12, 2021

hpatel41 Oct 13, 2021

kspeer825 left a comment

TDL-14896: Make batch_size apply to the first page and TDL-14895: Use the pagination__list_subscriber_interval_quantity config value correctly #64

Are you sure you want to change the base?

TDL-14896: Make batch_size apply to the first page and TDL-14895: Use the pagination__list_subscriber_interval_quantity config value correctly #64

Conversation

hpatel41 commented Aug 23, 2021

Description of change

Manual QA steps

Risks

Rollback steps

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbshah1212 Aug 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kspeer825 left a comment

Choose a reason for hiding this comment

dbshah1212 Aug 30, 2021 •

edited

Loading